Skip to content

Grid block refactor#105

Open
johngodley wants to merge 3 commits intomasterfrom
update/grid-css
Open

Grid block refactor#105
johngodley wants to merge 3 commits intomasterfrom
update/grid-css

Conversation

@johngodley
Copy link
Member

@johngodley johngodley commented Jun 25, 2020 •
edited
Loading

This is a big refactor that does several things:

  • It moves from span and offset to span and start - columns are absolutely positioned instead of being relative to the previous column. The user continues to see things in terms of offset
  • Columns now contain their own span and start attributes instead of storing everything in the grid container
  • Move to functional React with hooks instead of classes.

This results in many benefits:

  • Big reduction in code size and complexity. We no longer have to convert between start and offset all the time
  • CSS size is greatly reduced as we can apply the same CSS class to each column, instead of having to provide every possible combination on the parent
  • Possibility of using absolute positioning for future things (see Layout Grid: Allow overlapping of layers #63)
  • Should be easier to change how the grid resizing and snapping works

Other changes:

  • Column resizing is now squishy. That is, it will squish columns if you resize into them. This contrasts to the previous version where it would use spare-space, if available, but wouldn't squash. One effect is that if you squash a column down it doesn't expand back out if you change your mind - a problem?
  • It's no longer possible to move a column onto a different row. Previously, using the inspector only, you could modify the offset and span values by entering specific values that resulted in a column moving onto another row. I don't think anyone knew this was possible, and it wasn't very easy to use. Layout Grid: Allow overlapping of layers #63 and drag/drop columns could be a better way of changing columns, so I don't think the loss here is a problem

Upcoming changes:

  • Unit tests to check the grid continues to behave as expected
  • Block migration. The block attributes have changed, and we need to prevent invalidations. Ideally it can transparently accept old attributes and convert them to new ones when re-saved.
  • Further CSS size reduction. Maybe we can split out the IE prefix CSS into a separate IE only file?
  • Merge front.css and style.css together. I know we split them apart, but it makes integration difficult, and it would be better merged
  • Changing the layout auto-selects the first column. Not sure why, and it's annoying

@johngodley
Add vertical alignment to grid
0a7bcfc 
Adds a global alignment to the grid container, and individual alignments to the child columns

# Conflicts:
#	blocks/layout-grid/editor.scss
#	blocks/layout-grid/src/grid/edit.js
#	blocks/layout-grid/style.scss
@johngodley
Use a wider length
63c698e 
It’s 2020
@johngodley
Refactor grid to save settings in columns
4372c2b 
Each column now stores the settings, not the parent container. This allows us to reduce the CSS size, and overall complexity
@johngodley
Copy link
Member Author

johngodley commented Jun 25, 2020

@jasmussen this is still a work in progress and doesn't immediately need any attention, but just putting it on your radar.

@jasmussen
Copy link
Member

jasmussen commented Jul 1, 2020

This is a big one. I've received your ping, and it's on my ToDo, and I'll absolutely get to it. But I want to do it right.

@jasmussen
Copy link
Member

jasmussen commented Jul 10, 2020

I tested this today, and I must say, the amount of red code here is impressive.

The block itself is also working really well. In fact, it seems to be working perfectly, except for a few issues that I believe are not related to this branch specifically, but rather the block itself. But perhaps simply due to testing this heavily, they appeared.

The bad part is, I can no longer reproduce the issues, either in master or this branch. But nevertheless, I did encounter an issue where I switched from 3 to 4 columns, then pressed Undo, and the layout was messed up rather badly. Behold these values:

Screenshot 2020-07-10 at 12 05 57

I don't know if I didn't properly clear cache, but I could not recreate the conditions to cause this. But it did involve undo/redo and switching the amount of columns.

But in every case, even when the column values got broken for inexplicable reasons, the CSS seemed completely correct, insofar as the frontend always matched the backend.

What is the next step to moving forward with this one? Do we need some Takashi testing? Did you need CSS input?

@johngodley johngodley mentioned this pull request Apr 12, 2021
Merged
@johngodley johngodley mentioned this pull request May 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@johngodley @jasmussen